Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add adelais PCB #8945

Merged
merged 12 commits into from
May 9, 2020
Merged

add adelais PCB #8945

merged 12 commits into from
May 9, 2020

Conversation

mechlovin
Copy link
Contributor

@mechlovin mechlovin commented Apr 27, 2020 •
edited
Loading

Description

Add Adelais en ciel - compatible with TGR Alice and all clone variants

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@drashna drashna requested a review from a team April 29, 2020 09:01
fauxpark
fauxpark requested changes Apr 29, 2020
View reviewed changes
keyboards/mechlovin/adelais/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/mechlovin/adelais/rules.mk Outdated Show resolved Hide resolved
@drashna drashna requested a review from a team April 29, 2020 16:51
@mechlovin mechlovin requested review from fauxpark and removed request for a team April 30, 2020 03:36
@fauxpark
Copy link
Member

What's the difference between this and #8944?

@mechlovin mechlovin closed this Apr 30, 2020
@mechlovin mechlovin reopened this Apr 30, 2020
@mechlovin
Copy link
Contributor Author

this is standard backlighting (i mean soldering LED), #8944 is rgb pcb

@mechlovin
Copy link
Contributor Author

could i merge them?

@fauxpark
Copy link
Member

fauxpark commented Apr 30, 2020 •
edited
Loading

So is it the exact same PCB?

If so, adding two keyboards isn't the best way to do this. You can just enable both features in the same config. Since this is using an ARM chip, there's plenty of flash space to accommodate it.

@mechlovin
Copy link
Contributor Author

Oh, got it. Thank you.
May i close this PR, #8944 and open new PR?

@fauxpark
Copy link
Member

You don't need to open a new PR. Just use one of these two and close the other.

@mechlovin
Copy link
Contributor Author

okie, thank you!

@fauxpark
Copy link
Member

The config.h and rules.mk stuff can be deduped here, you don't need separate keymaps for the features.

@fauxpark
Copy link
Member

Oh, I see, there are two versions of this PCB after all, one with RGB matrix, and one with underglow + backlight. So, instead of separate keymaps you should make subfolders inside the adelais folder for each version with just the relevant config/rules stuff. Anything that's common to both can be in the top-level rules.mk/config.h.

@mechlovin
Copy link
Contributor Author

Exactly, these are the 2 PCB versions. But they have the same layout, matrix and pinout.
So I think what I changed is fine :D

@mechlovin
Copy link
Contributor Author

Hi @fauxpark, Do I have to change anything else?

@fauxpark
Copy link
Member

fauxpark commented May 2, 2020

As I said, don't use separate keymaps for this. Make subfolders with the different config options, and leave stuff common to both in the top-level config/rules.

@mechlovin
Copy link
Contributor Author

I just updated according to what I understand.
Am I doing it right?
Thank you! :D

fauxpark
fauxpark requested changes May 2, 2020
View reviewed changes
Copy link
Member

@fauxpark fauxpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking better. The keymaps dirs seem identical so they can be pulled up as well.

keyboards/mechlovin/adelais/config.h Outdated Show resolved Hide resolved
keyboards/mechlovin/adelais/rgb_led/config.h Outdated Show resolved Hide resolved
keyboards/mechlovin/adelais/standard_led/config.h Outdated Show resolved Hide resolved
keyboards/mechlovin/adelais/standard_led/readme.md Outdated Show resolved Hide resolved
keyboards/mechlovin/adelais/rgb_led/readme.md Outdated Show resolved Hide resolved
keyboards/mechlovin/adelais/readme.md Outdated Show resolved Hide resolved
keyboards/mechlovin/adelais/readme.md Outdated Show resolved Hide resolved
@mechlovin mechlovin requested a review from fauxpark May 3, 2020 06:55
fauxpark
fauxpark requested changes May 3, 2020
View reviewed changes
Copy link
Member

@fauxpark fauxpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the rgb_led keymaps folder under the keyboard folder, and delete the standard_led keymaps folder.

@fauxpark fauxpark requested review from drashna and a team May 3, 2020 07:10
@mechlovin
Copy link
Contributor Author

Hi @drashna. Could you please help me to check them again?

noroadsleft
noroadsleft requested changes May 7, 2020
View reviewed changes
keyboards/mechlovin/adelais/info.json Outdated Show resolved Hide resolved
keyboards/mechlovin/adelais/info.json Show resolved Hide resolved
@mechlovin mechlovin requested a review from noroadsleft May 8, 2020 14:24
@mechlovin mechlovin mentioned this pull request May 8, 2020
Merged
6 tasks
noroadsleft
noroadsleft approved these changes May 8, 2020
View reviewed changes
Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@drashna drashna merged commit 4c4009c into qmk:master May 9, 2020
bitherder pushed a commit to bitherder/qmk_firmware that referenced this pull request May 15, 2020
@mechlovin @vuhopkep @lorenzoernesto
[Keyboard] add adelais PCB (qmk#8945)
1d0cf01 
* add adelais PCB

* clean up

* merger

* update

* Update readme.md

* Update config.h

* update

* update

* move keymap folder

* Update info.json

* Update info.json

* Update info.json

Co-authored-by: vuhopkep <[email protected]>
sowbug pushed a commit to sowbug/qmk_firmware that referenced this pull request May 24, 2020
@mechlovin @vuhopkep @sowbug
[Keyboard] add adelais PCB (qmk#8945)
bcf8d72 
* add adelais PCB

* clean up

* merger

* update

* Update readme.md

* Update config.h

* update

* update

* move keymap folder

* Update info.json

* Update info.json

* Update info.json

Co-authored-by: vuhopkep <[email protected]>
@mechlovin mechlovin deleted the dev_adelais branch May 30, 2020 02:53
turky pushed a commit to turky/qmk_firmware that referenced this pull request Jun 13, 2020
@mechlovin @vuhopkep @turky
[Keyboard] add adelais PCB (qmk#8945)
242daa9 
* add adelais PCB

* clean up

* merger

* update

* Update readme.md

* Update config.h

* update

* update

* move keymap folder

* Update info.json

* Update info.json

* Update info.json

Co-authored-by: vuhopkep <[email protected]>
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
@mechlovin @vuhopkep @jakobaa
[Keyboard] add adelais PCB (qmk#8945)
db4b4b9 
* add adelais PCB

* clean up

* merger

* update

* Update readme.md

* Update config.h

* update

* update

* move keymap folder

* Update info.json

* Update info.json

* Update info.json

Co-authored-by: vuhopkep <[email protected]>
sjmacneil pushed a commit to sjmacneil/qmk_firmware that referenced this pull request Feb 19, 2021
@mechlovin @vuhopkep @sjmacneil
[Keyboard] add adelais PCB (qmk#8945)
bbffe7a 
* add adelais PCB

* clean up

* merger

* update

* Update readme.md

* Update config.h

* update

* update

* move keymap folder

* Update info.json

* Update info.json

* Update info.json

Co-authored-by: vuhopkep <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noroadsleft noroadsleft noroadsleft approved these changes

@fauxpark fauxpark fauxpark approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
awaiting review keyboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mechlovin @fauxpark @drashna @noroadsleft @vuhopkep